Make artifact URL prefix configurable#13367
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes the K3s artifact URL prefix configurable by introducing the INSTALL_K3S_ARTIFACT_URL environment variable, replacing the previous GITHUB_URL variable. This allows K3s to be downloaded from locations other than GitHub, such as private mirrors or alternative artifact repositories. The default behavior remains unchanged, pointing to the official GitHub releases.
Key changes:
- Added
INSTALL_K3S_ARTIFACT_URLenvironment variable with documentation - Implemented URL-safe encoding for version strings containing special characters (e.g., "+" becomes "%2B")
- Updated artifact download URLs to use the new configurable prefix
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
This should work for public endpoints, but what if Could possibly use support for specifying local artifact via filesystem path |
|
We don't need to solve all possibile problems now, just add parity with what the rke2 install script can do. |
manuelbuil
force-pushed
the
k3sartifacturl
branch
2 times, most recently
from
c278d55 to
9dd78ab
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
manuelbuil
force-pushed
the
k3sartifacturl
branch
3 times, most recently
from
2941985 to
f196f49
Compare
|
|
||
| # GITHUB_URL was replaced by INSTALL_K3S_ARTIFACT_URL | ||
| if [ -n "$GITHUB_URL" ]; then | ||
| echo "WARNING: 'GITHUB_URL' is deprecated and will be removed in a future release." >&2 |
There was a problem hiding this comment.
This isn't accurate - deprecated would indicate that it is outdated but still works. It doesn't work and support for it has already been removed.
Either make it work again, or correct the warning to indicate that support for it is CURRENTLY removed.
manuelbuil
force-pushed
the
k3sartifacturl
branch
from
f196f49 to
f9b02d4
Compare
|
|
||
| # GITHUB_URL was replaced by INSTALL_K3S_ARTIFACT_URL | ||
| if [ -n "$GITHUB_URL" ]; then | ||
| echo "WARNING: 'GITHUB_URL' does not work anymore. Please use 'INSTALL_K3S_ARTIFACT_URL' instead." >&2 |
There was a problem hiding this comment.
We have info/warn/error helpers, use those:
| echo "WARNING: 'GITHUB_URL' does not work anymore. Please use 'INSTALL_K3S_ARTIFACT_URL' instead." >&2 | |
| warn "'GITHUB_URL' does not work anymore. Please use 'INSTALL_K3S_ARTIFACT_URL' instead." |
You'll need to move this down below the function definitions to do that - perhaps next to the VERSION_URLSAFE on line 390?
There was a problem hiding this comment.
Makes sense. But why so far away? Warn is the 2nd function that gets defined, I could add it below it. Let me know what you think
There was a problem hiding this comment.
the convention in this script is to define logic in functions, and then call those functions. Having this test+warn bit outside a function breaks that convention.
There was a problem hiding this comment.
ah, that's a good point. We should follow convention. I'm going to add it in the "download_and_verify()" function right before "download_hash", which is the first place where GITHUB_URL was used.
I don't add it in "download_hash" function because "download_binary" also used GITHUB_URL, and it might be confusing if it is only in one of the functions. I'm also not creating a new function just for that logic, since it's three lines and just adding a log. Tell me what you think, I'm ok with changing that
manuelbuil
force-pushed
the
k3sartifacturl
branch
from
f9b02d4 to
bf37526
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Manuel Buil <mbuil@suse.com>
manuelbuil
force-pushed
the
k3sartifacturl
branch
from
bf37526 to
16d81d3
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13367 +/- ##
==========================================
- Coverage 21.42% 21.36% -0.07%
==========================================
Files 191 191
Lines 15508 15513 +5
==========================================
- Hits 3323 3314 -9
- Misses 11729 11751 +22
+ Partials 456 448 -8
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Proposed Changes
Allow setting INSTALL_K3S_ARTIFACT_URL to override URL prefix for release artifacts, to allow downloading K3s from locations other than GitHub.
Breaking Variable Changes! It replaces the env var GITHUB_URL which was doing a similar thing but required the repo to be in Github
Types of Changes
New feature
Verification
Taking the install.sh script:
Testing
Linked Issues
#13366
User-Facing Change
Further Comments